backend/app/services/evaluations/evaluation.py-145-152 (1)

145-152: ⚠️ Potential issue | 🟡 Minor

Unhandled KeyError if config.completion.type is unexpected.

If config.completion.type holds a value not in the param_models dict, line 151 raises a bare KeyError, which is caught by the generic except on line 169 but produces a cryptic error message. Consider using .get() with an explicit error.

Additionally, evaluations currently only support the OpenAI provider (line 109), but there's no guard against non-text types. An STT/TTS config would pass provider validation yet produce invalid evaluation JSONL for /v1/responses.

Proposed fix

-        model_class = param_models[config.completion.type]
+        model_class = param_models.get(config.completion.type)
+        if model_class is None:
+            raise HTTPException(
+                status_code=422,
+                detail=f"Unsupported completion type for evaluation: '{config.completion.type}'. "
+                       f"Only 'text' is currently supported.",
+            )
         validated_params = model_class.model_validate(config.completion.params)

backend/app/tests/api/routes/configs/test_version.py-477-477 (1)

477-477: ⚠️ Potential issue | 🟡 Minor

Unused import: TextLLMParams.

TextLLMParams is imported but never used in this test function.

-    from app.models.llm.request import KaapiCompletionConfig, TextLLMParams
+    from app.models.llm.request import KaapiCompletionConfig

backend/app/models/llm/response.py-7-8 (1)

7-8: ⚠️ Potential issue | 🟡 Minor

Unused imports: base64 and model_validator.

Neither base64 nor model_validator is referenced anywhere in this file.

-import base64
-from pydantic import model_validator
 from sqlmodel import SQLModel, Field

backend/app/tests/utils/test_data.py-339-358 (1)

339-358: ⚠️ Potential issue | 🟡 Minor

config_type can be None if "type" key is missing from the stored config blob.

Line 342 uses .get("type") which returns None if the key is absent. Passing None to NativeCompletionConfig(type=config_type) or KaapiCompletionConfig(type=config_type) will fail validation since type is Literal["text", "stt", "tts"].

Proposed fix — add a default

-            config_type = completion_config.get("type")
+            config_type = completion_config.get("type", "text")

backend/app/services/llm/providers/registry.py-71-71 (1)

71-71: ⚠️ Potential issue | 🟡 Minor

Log message missing function name prefix.

Per coding guidelines, log messages should be prefixed with the function name in square brackets.

Proposed fix

-        logger.error(f"Failed to initialize {provider_type} client: {e}", exc_info=True)
+        logger.error(f"[get_llm_provider] Failed to initialize {provider_type} client: {e}", exc_info=True)

As per coding guidelines, "Prefix all log messages with the function name in square brackets."

backend/app/services/llm/providers/gai.py-118-118 (1)

118-118: ⚠️ Potential issue | 🟡 Minor

Remove extraneous f prefix from f-strings with no placeholders.

Static analysis (Ruff F541) flags lines 118 and 272 as f-strings that contain no interpolated expressions.

Proposed fix

-            f"[GoogleAIProvider._execute_stt] Response missing usage_metadata, using zeros"
+            "[GoogleAIProvider._execute_stt] Response missing usage_metadata, using zeros"

-            f"[GoogleAIProvider._execute_tts] Response missing usage_metadata, using zeros"
+            "[GoogleAIProvider._execute_tts] Response missing usage_metadata, using zeros"

Also applies to: 272-272

backend/app/crud/llm.py-39-47 (1)

39-47: ⚠️ Potential issue | 🟡 Minor

size_bytes records the base64 string length, not the actual audio byte count.

len(query_input.content.value) where value is a base64-encoded string gives the character count of the encoding, which is ~33% larger than the actual audio data. If the intent is to store real audio size, decode first (or multiply by 3/4). If the intent is just to store the payload size, rename to base64_length to avoid confusion with the decoded-size calculation in update_llm_call_response (line 186-187).

backend/app/models/llm/request.py-180-182 (1)

180-182: ⚠️ Potential issue | 🟡 Minor

Stale description: "LLM provider (openai)" should reflect the expanded provider set.

The field description still says (openai) but the Literal now includes "google" as well.

Proposed fix

     provider: Literal["openai", "google"] = Field(
-        ..., description="LLM provider (openai)"
+        ..., description="LLM provider (openai, google)"
     )

backend/app/crud/llm.py-78-80 (1)

78-80: ⚠️ Potential issue | 🟡 Minor

getattr fallback on a dict will never resolve "type" correctly.

completion_config.params is dict[str, Any] (per both NativeCompletionConfig and KaapiCompletionConfig). getattr(a_dict, "type", "text") checks for a Python attribute named type on the dict object, not a dict key — it would always return "text" regardless of the dict's contents. Per a retrieved learning, the type field on completion_config is mandatory, so completion_config.type should always be set and this fallback is effectively dead code. If the fallback is still desired for safety, use .get().

Proposed fix

-    completion_type = completion_config.type or getattr(
-        completion_config.params, "type", "text"
-    )
+    completion_type = completion_config.type

Based on learnings: "the type field in config_blob.completion (for both NativeCompletionConfig and KaapiCompletionConfig) is compulsory/mandatory during config creation and validation ensures it will never be missing."

backend/app/tests/crud/test_llm.py-42-46 (1)

42-46: ⚠️ Potential issue | 🟡 Minor

Missing return type annotation on test_job fixture.

All other fixtures and test functions include return type hints. This fixture is missing one.

As per coding guidelines: "Always add type hints to all function parameters and return values in Python code."

Proposed fix

 `@pytest.fixture`
-def test_job(db: Session):
+def test_job(db: Session) -> Any:
     """Create a test job for LLM call tests."""
     crud = JobCrud(db)
     return crud.create(job_type=JobType.LLM_API, trace_id="test-llm-trace")

Replace Any with the actual Job type if available — likely from app.models import Job would be the proper return type.

backend/app/services/llm/mappers.py-98-102 (1)

98-102: ⚠️ Potential issue | 🟡 Minor

model is unconditionally set (possibly to None) unlike the OpenAI mapper which guards with if model.

Line 102 sets google_params["model"] = kaapi_params.get("model") which could store None. The OpenAI mapper (line 61-62) only sets model when truthy. Consider aligning the behavior or adding a guard.

Proposed fix

-    # Model is present in all param types
-    google_params["model"] = kaapi_params.get("model")
+    model = kaapi_params.get("model")
+    if not model:
+        raise ValueError("Missing required 'model' parameter in kaapi_params")
+    google_params["model"] = model

backend/app/crud/llm.py-95-99 (1)

95-99: ⚠️ Potential issue | 🟡 Minor

hasattr on a dict won't find the "model" key — always falls to the else branch.

completion_config.params is a plain dict. hasattr(dict_instance, "model") checks for a Python attribute, not a dict key, so it will always be False. The else branch with .get("model", "") is the one that actually executes. Simplify to just the .get() call.

Proposed fix

-    model = (
-        completion_config.params.model
-        if hasattr(completion_config.params, "model")
-        else completion_config.params.get("model", "")
-    )
+    model = completion_config.params.get("model", "")

backend/app/models/llm/request.py-9-11 (1)

9-11: ⚠️ Potential issue | 🟡 Minor

Duplicate imports: Field and SQLModel are imported twice.

Line 4 imports Field, SQLModel from sqlmodel, and line 11 reimports them along with Index, text. Ruff (F811) correctly flags this. Consolidate into a single import.

Proposed fix

-from sqlmodel import Field, SQLModel
 from pydantic import Discriminator, model_validator, HttpUrl
 from datetime import datetime
 from app.core.util import now
 
 import sqlalchemy as sa
 from sqlalchemy.dialects.postgresql import JSONB
-from sqlmodel import Field, SQLModel, Index, text
+from sqlmodel import Field, SQLModel, Index, text

backend/app/services/llm/providers/gai.py-230-257 (1)

230-257: ⚠️ Potential issue | 🟡 Minor

Fix misleading comment about Gemini TTS output format.

Line 231 incorrectly states the output is "PCM (WAV format)" — Gemini TTS returns raw PCM bytes without a WAV container. The WAV header is added client-side if needed. Change the comment to clarify: "Gemini TTS natively outputs 24kHz 16-bit PCM (raw audio data, no WAV header)". The conversion logic itself is correct and properly handles raw PCM input.

backend/app/tests/seed_data/seed_data.py (1)

90-97: Consider updating log message prefixes to use individual function names.

All log messages currently use the generic [tests.seed_data] prefix. This makes it harder to filter logs by specific functions during debugging. As per coding guidelines, each function should prefix logs with its own name: [load_seed_data], [create_organization], [clear_database], etc.

Example fix for load_seed_data

     except FileNotFoundError:
         logging.error(
-            f"[tests.seed_data]Error: Seed data file not found at {json_path}"
+            f"[load_seed_data] Error: Seed data file not found at {json_path}"
         )
         raise
     except json.JSONDecodeError as e:
         logging.error(
-            f"[tests.seed_data]Error: Failed to decode JSON from {json_path}: {e}"
+            f"[load_seed_data] Error: Failed to decode JSON from {json_path}: {e}"
         )

As per coding guidelines: "Prefix all log messages with the function name in square brackets."

Also applies to: 110-110, 137-137, 157-157, 212-212, 254-254, 293-293, 344-344, 359-359, 389-389, 418-418, 438-438, 479-479, 481-481

backend/app/services/llm/input_resolver.py (1)

33-33: Remove or replace the # important!! comment.

This comment doesn't convey any useful information. Either remove it or replace with a meaningful note about why this function is critical.

Proposed fix

-# important!!
 def resolve_input(query_input: QueryInput) -> tuple[str, str | None]:

backend/app/tests/services/llm/providers/test_registry.py (1)

23-26: Consider adding a test for the google-native registry entry.

test_registry_contains_openai explicitly asserts "openai-native" is in the registry and maps to OpenAIProvider. With the new google-native provider, a parallel assertion for GoogleAIProvider would prevent regressions on the new provider wiring.

backend/app/tests/api/routes/configs/test_config.py (1)

49-59: Consider asserting max_tokens is stripped.

The comment on line 49 states that "invalid fields like max_tokens are stripped," but the assertions only verify that valid fields are present. Adding a negative assertion (e.g., assert "max_tokens" not in ...params) would strengthen this test.

backend/app/models/config/version.py (1)

99-116: Missing empty config_blob validation.

ConfigVersionBase validates that config_blob is non-empty via validate_blob_not_empty, but ConfigVersionUpdatePartial doesn't inherit from it and lacks this check. A request with config_blob: {} would pass model validation. Consider adding the same validator or inheriting it.

Proposed fix

+from pydantic import field_validator
+
 class ConfigVersionUpdatePartial(SQLModel):
     ...
 
     config_blob: dict[str, Any] = Field(
         description="Partial config blob. Only include fields you want to update. "
         "Provider and type are inherited from existing config and cannot be changed.",
     )
     commit_message: str | None = Field(
         default=None,
         max_length=512,
         description="Optional message describing the changes in this version",
     )
+
+    `@field_validator`("config_blob")
+    def validate_blob_not_empty(cls, value: dict[str, Any]) -> dict[str, Any]:
+        if not value:
+            raise ValueError("config_blob cannot be empty")
+        return value

backend/app/api/routes/config/version.py (1)

25-30: Missing return type annotation on create_version.

As per coding guidelines, all function parameters and return values should have type hints.

Proposed fix

 def create_version(
     config_id: UUID,
     version_create: ConfigVersionUpdatePartial,
     current_user: AuthContextDep,
     session: SessionDep,
-):
+) -> APIResponse[ConfigVersionPublic]:

backend/app/services/llm/providers/PLAN.md (1)

1-88: Consider whether this planning document should be committed to the repository.

This file reads like an internal implementation specification / prompt for development. If it's meant to be permanent documentation, it should live in a docs/ directory with proper formatting. If it was used only during development, consider removing it before merge.

A few text issues if keeping it:

• Line 84: thorugh → through
• Line 84: providers.gai.py → providers/gai.py (wrong separator)
• Line 88: over abstracted → over-abstracted

backend/app/tests/api/routes/configs/test_version.py (1)

522-620: Inconsistent assertion depth across type-change tests.

test_create_version_cannot_change_type_from_text_to_stt (Line 516-519) verifies the error message content ("cannot change config type", "text", "stt"), but the stt→tts and tts→text variants only assert status_code == 400 without checking the error detail. Consider adding the same error message assertions for consistency and to catch regressions in error messaging.

backend/app/alembic/versions/045_add_llm_call_table.py (2)

161-168: Redundant alter_column on a table created in the same migration.

Lines 22-138 create llm_call with provider comment set to "AI provider: openai, google, anthropic", then lines 161-168 immediately alter it to a different comment. Set the correct comment directly in create_table and remove this alter_column. Similarly, the downgrade (lines 182-189) alters this column's comment right before dropping the table — also unnecessary.

Proposed fix in create_table

         sa.Column(
             "provider",
             sa.String(),
             nullable=False,
-            comment="AI provider: openai, google, anthropic",
+            comment="AI provider as sent by user (e.g openai, -native, google)",
         ),

Then remove the alter_column blocks for llm_call.provider in both upgrade() and downgrade().

---

22-152: Consider adding an index on project_id for query performance.

The table has foreign keys on job_id, project_id, and organization_id, but only job_id and conversation_id are indexed. If LLM calls are frequently queried by project (e.g., listing calls for a project), a partial index on project_id (filtered by deleted_at IS NULL) would improve performance.

backend/app/tests/services/llm/test_mappers.py (2)

9-15: TTSLLMParams is imported but unused in this test file.

No tests exercise TTS-specific parameter mapping for Google (e.g., voice, language fields). Given this PR introduces Gemini TTS, consider adding a test using TTSLLMParams to verify map_kaapi_to_google_params correctly maps voice and language parameters.

Example test for TTS param mapping

def test_tts_params_mapping(self):
    """Test TTS-specific parameter mapping (voice, language)."""
    kaapi_params = TTSLLMParams(
        model="gemini-2.5-pro-preview-tts",
        voice="Kore",
        language="hi-IN",
    )

    result, warnings = map_kaapi_to_google_params(
        kaapi_params.model_dump(exclude_none=True)
    )

    assert result["model"] == "gemini-2.5-pro-preview-tts"
    assert result["voice"] == "Kore"
    assert result["language"] == "hi-IN"
    assert warnings == []

---

457-496: Consider adding a TTS-specific transform test for Google.

The Google transform tests cover type="stt" (line 461) and type="text" (line 482) but not type="tts", which is the primary use case this PR introduces.

backend/app/tests/services/llm/test_jobs.py (1)

18-22: Remove the commented-out import.

Line 20 has # KaapiLLMParams left as a comment. Clean up dead code.

Proposed fix

     TextOutput,
     TextContent,
-    # KaapiLLMParams,
     KaapiCompletionConfig,
 )

backend/app/services/llm/jobs.py (1)

253-255: Remove else: pass — it's dead code.

Proposed fix

                     request.request_metadata.setdefault("warnings", []).extend(warnings)
-                else:
-                    pass
             except Exception as e:

backend/app/tests/utils/test_data.py (1)

23-23: Remove unused KaapiLLMParams import.

The code constructs configs with dict-based params directly. KaapiLLMParams is imported but never referenced in the file.

Proposed fix

-from app.models.llm import KaapiLLMParams, KaapiCompletionConfig, NativeCompletionConfig
+from app.models.llm import KaapiCompletionConfig, NativeCompletionConfig

backend/app/crud/llm.py (1)

10-17: Nit: group stdlib imports together.

base64 and json (stdlib) are separated from logging, UUID, etc. by third-party imports. Minor readability nit.

Proposed grouping

 import logging
+import base64
+import json
 from typing import Any, Literal
-
 from uuid import UUID
+
 from sqlmodel import Session, select
 from app.core.util import now
-import base64
-import json
 from app.models.llm import LlmCall, LLMCallRequest, ConfigBlob

backend/app/crud/config/version.py (2)

291-341: _validate_config_type_unchanged duplicates _get_latest_version.

Lines 300-311 replicate the same query that _get_latest_version() (lines 155-168) already encapsulates. Consider reusing _get_latest_version() here.

Proposed refactor

     def _validate_config_type_unchanged(
         self, version_create: ConfigVersionCreate
     ) -> None:
-        # Get the latest version
-        stmt = (
-            select(ConfigVersion)
-            .where(
-                and_(
-                    ConfigVersion.config_id == self.config_id,
-                    ConfigVersion.deleted_at.is_(None),
-                )
-            )
-            .order_by(ConfigVersion.version.desc())
-            .limit(1)
-        )
-        latest_version = self.session.exec(stmt).first()
+        latest_version = self._get_latest_version()
 
         # If this is the first version, no validation needed
         if latest_version is None:
             return

---

34-76: Both create_or_raise and create_from_partial_or_raise share identical version-creation boilerplate.

The try/except block that calls _get_next_version, constructs a ConfigVersion, commits, and handles errors is duplicated across both methods (lines 45-76 and 122-153). Consider extracting a private _persist_version(config_blob_dict, commit_message) helper.

Also applies to: 78-153

backend/app/services/llm/mappers.py (1)

42-50: Temperature is always suppressed when the model supports reasoning, even if no reasoning param was provided.

If a user sends only temperature (no reasoning param) for a reasoning-capable model, the temperature is silently dropped with a warning. This may be intentional per OpenAI API constraints, but it could surprise users who didn't opt into reasoning. Consider only suppressing temperature when reasoning is actually set.

backend/app/services/llm/providers/gai.py (4)

196-198: Move from google.genai import types to module-level imports.

Inline imports inside a method body can hide dependencies and incur repeated import overhead on each call. Since google.genai is already imported at the module level (line 3), move this import to the top of the file.

Proposed fix

At the top of the file:

 from google import genai
 from google.genai.types import GenerateContentResponse
+from google.genai import types

Inside _execute_tts, remove:

-        from google.genai import types

---

25-32: Redundant self.client = client — already set by super().__init__(client).

BaseProvider.__init__ already assigns self.client = client (per the base class snippet). The explicit reassignment on line 32 is unnecessary.

Proposed fix

     def __init__(self, client: genai.Client):
         super().__init__(client)
-        self.client = client

---

340-350: Broad except Exception swallows the original error message from the caller.

Line 346 sets error_message = "Unexpected error occurred" discarding the actual exception text. While the detailed error is logged, the caller only sees a generic message. This makes debugging harder for API consumers. Consider including a sanitized error summary.

---

91-97: Uploaded file resource is never cleaned up.

self.client.files.upload(file=resolved_input) uploads the audio file to Google's storage, but there's no finally block or cleanup call (e.g., self.client.files.delete(name=gemini_file.name)) after the transcription completes. Over time this could accumulate orphaned files in the Google AI storage.